Skip to content

[Validator] Email - new allow-no-tld mode #17360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

guillemfondin
Copy link
Contributor

See Related feature PR

As suggested here ; the actual description of html5 mode isn't consistent with the one provided by browsers. Browsers allow no tld, html5 mode not.

I proprose to move the actual description of html5 mode to the new allow-no-tld mode description, and clarify the subtlety (force .tld) of the actual html5 mode.

@guillemfondin guillemfondin requested a review from xabbuh as a code owner October 17, 2022 17:25
@carsonbot carsonbot added this to the 6.2 milestone Oct 17, 2022
@OskarStark OskarStark added the Waiting Code Merge Docs for features pending to be merged label Oct 17, 2022
@carsonbot carsonbot modified the milestones: 6.2, next Oct 17, 2022
fabpot added a commit to symfony/symfony that referenced this pull request Oct 20, 2022
…w3c official specification (guillemfondin)

This PR was squashed before being merged into the 6.2 branch.

Discussion
----------

[Validator] new email validation option to match with w3c official specification

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #47712
| License       | MIT
| Doc PR        | symfony/symfony-docs#17360

## WHAT
Email validator no matches with w3c official specication (that allow no tld) see #47712 for details.

## WHY
In general, for public common usage, developers may not need to give users this liberty.
Except for specific use case, (internal email for eg) in companies...

## HOW
Add an option for accept tld (already exist `Email::VALIDATION_MODE_HTML5`) or not (official spec `Email::VALIDATION_MODE_HTML5_ALLOW_NO_TLD`)

Commits
-------

23047d9 [Validator] new email validation option to match with w3c official specification
symfony-splitter pushed a commit to symfony/validator that referenced this pull request Oct 20, 2022
…w3c official specification (guillemfondin)

This PR was squashed before being merged into the 6.2 branch.

Discussion
----------

[Validator] new email validation option to match with w3c official specification

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #47712
| License       | MIT
| Doc PR        | symfony/symfony-docs#17360

## WHAT
Email validator no matches with w3c official specication (that allow no tld) see #47712 for details.

## WHY
In general, for public common usage, developers may not need to give users this liberty.
Except for specific use case, (internal email for eg) in companies...

## HOW
Add an option for accept tld (already exist `Email::VALIDATION_MODE_HTML5`) or not (official spec `Email::VALIDATION_MODE_HTML5_ALLOW_NO_TLD`)

Commits
-------

23047d978f [Validator] new email validation option to match with w3c official specification
@xabbuh xabbuh modified the milestones: next, 6.2 Oct 20, 2022
@xabbuh xabbuh removed the Waiting Code Merge Docs for features pending to be merged label Oct 20, 2022
Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

We should also add a .. versionadded:: 6.2 block after the list to indicate that this mode was introduced in 6.2.

@wouterj wouterj force-pushed the feature/new-standard-email-validation branch from 0bda552 to 485d57e Compare October 23, 2022 13:39
@wouterj
Copy link
Member

wouterj commented Oct 23, 2022

Thank you for documenting your code contribution, @guillemfondin!

@wouterj wouterj merged commit 6087ab6 into symfony:6.2 Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants